-
Notifications
You must be signed in to change notification settings - Fork 981
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fixes #27931 - 'Could not match network interface' #7066
Conversation
Can one of the admins verify this patch? |
2 similar comments
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Issues: #27931 |
There were the following issues with the commit message:
If you don't have a ticket number, please create an issue in Redmine. More guidelines are available in Coding Standards or on the Foreman wiki. This message was auto-generated by Foreman's prprocessor |
ok to test |
@@ -40,7 +40,7 @@ def scsi_controller_type | |||
def select_nic(fog_nics, nic) | |||
nic_attrs = nic.compute_attributes | |||
all_networks = service.list_networks(datacenter: datacenter) | |||
vm_network = all_networks.detect { |network| nic_attrs['network'] && [network[:name], network[:id], network[:_ref]].compact.include?(nic_attrs['network']) } | |||
vm_network = all_networks.detect { |network| nic_attrs['network'] && [network[:name], network[:id]].compact.include?(nic_attrs['network']) } | |||
vm_network ||= all_networks.detect { |network| network[:_ref] == nic_attrs['network'] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't mind droping the _ref
even here, but could we at least move the name here as well, so we know the id
is the primary attribute?
vm_network = all_networks.detect { |network| network[:id] == nic_attrs['network'] }
vm_network ||= all_networks.detect { |network| nic_attrs['network'] && [network[:name], network[:_ref]].compact.include?(nic_attrs['network']) }
Or even drop the _ref
completely and make both lines simple.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't feel like I understand this code well enough to make a judgement on the proposed change nor to test it in any way that would confirm if either way is better. The root cause I can find is that some of the objects in our vCenter have _ref's that match other object's id's. Any fix which ensures a positive match on id is always selected in preference to any possible match against _ref should fix the issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood, this change make sense as is.
I will propose the cleanup in followup PR, if you are uncomfortable to do it ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @pendor, nice cleanup!
I'll wait for jenkins and merge :)
Failure unrelated. |
@tbrisker could we backport it at least to 1.23 ? |
1.23- 0dca76b |
This fixes failure creating a VMware backed host, possibly only where vCenter was upgraded from 6.0 or earlier. The original issue is #23909 which was fixed in f5ea70a and regressed in 5150a1d.